fix(core,producer): stamp render ids on empty-src media and pair the snapshot by them - #3513

🟢 LGTM from my side — leaving as a comment.

The fix maps cleanly to #3512's root cause and lines up with every downstream consumer I traced. Small, precise, low blast radius.

Root-cause fit — yes

#3512 traces the collapse to two facts stacked: (a) #3342 only stamped data-hf-render-id on elements that already had a playable source, so <video id="clip" src=""> + runtime el.src = url was skipped; (b) discoverMediaFromBrowser then keyed the snapshot by author id, and the two id="clip" scenes collapsed onto one window. This PR removes hasPlayableSource (packages/core/src/compiler/mediaRenderIds.ts — the whole hasPlayableSource helper and the !hasPlayableSource(el) continue; gate) and threads the render id through the three snapshot consumers (htmlCompiler.ts:2162, :2226, :2317, :2436). Both halves of the failure are addressed at their source — no papering-over.

Bridge is wired everywhere it needs to be

The __hfMediaEl resolvers in the two new lookup sites (htmlCompiler.ts:2226, :2317) rely on installMediaRenderIdBridge running before any page script. I checked: the bridge is installed inside constructCaptureSession at packages/engine/src/services/frameCapture.ts:1337, which is what createCaptureSession at packages/producer/src/services/render/stages/probeStage.ts:309 and :418 calls. So session.page used by all three discover* functions always has __hfMediaEl available. The ?? document.getElementById(id) fallback is a safety net for direct-engine callers, but on the actual render path the bridge is guaranteed. Good.

Cross-consumer consistency — all render-id-aware

Grepped every data-hf-render-id / MEDIA_RENDER_ID_ATTR consumer and all of them prefer the render id over the raw id:

• packages/producer/src/services/audioExtractor.ts:73 — renderIdMatch?.[1] || idMatch?.[1], so the static-parse path already emits render ids when present.
• packages/producer/src/services/renderMediaCollector.ts:96–97 — enumerates by [data-hf-render-id].
• packages/engine/src/services/videoFrameExtractor.ts:571,645, audioMixer.ts:559, screenshotService.ts:480,516,533, videoFrameInjector.ts:296,322, frameCapture.ts:2006, core/src/runtime/renderFrameSibling.ts:26–27, core/src/audioGroups.ts:101,152 — all prefer render id, fall back to .id.

Net: no two-ID-systems split. This PR is the last threading gap being closed.

Merge into composition.videos / .audios is coherent

The probeStage merge at probeStage.ts:481–540 keys existingVideoIds on composition.videos[i].id. For the failure case in #3512, static parse skips (empty src) so composition.videos is empty pre-merge and every browser-discovered entry enters via the "new video" branch keyed by its unique render id (clip, clip__hf2). No collision. For mixed cases (one scene static-src, one scene runtime-src, same author id), audioExtractor.ts:73 guarantees the static-side already emits render ids too, so the sets align on the same key namespace. Verified: compiled.videos in the new htmlCompiler.test.ts at line 2846 asserts toHaveLength(0) for the empty-src case, matching that model.

Idempotency preserved

assignMediaRenderIds at mediaRenderIds.ts:101–108 still short-circuits on existing render id and adds it to taken, so the resolved-durations recompile path won't renumber in-flight extractions. Docstring at line 92–96 remains accurate.

Backwards-compat / scope

Nothing persisted — this is a compile-time attribute stamp on the render document. No migration surface, no cached-artifact invalidation. <img> intentionally still requires the src attribute (empty allowed), which is documented at mediaRenderIds.ts:49–51. Deliberate scoping to video/audio; not a bug.

CI

All required checks green at 874b323: Producer: unit + integration, Test, Test: runtime contract, Preview parity, Perf: parity/scrub/load/fps/drift, Regression shards 1–9, Tests on windows-latest, Render on windows-latest, Studio: timeline viewport gate, Typecheck, Lint, Format, CLI smoke (required). mergeStateStatus=BLOCKED is just REVIEW_REQUIRED; mergeable=MERGEABLE, no auto-merge armed.

No prior peer reviews. GET /repos/heygen-com/hyperframes/pulls/3513/reviews returns [], no issue comments either. This is the first eyes.

Nits — none blocking

• Missing audio-analogue test. mediaRenderIds.test.ts:78–91 covers the collision case for <video> only. <audio id="bed" src=""> in two nested scenes is symmetric under the widened selector, and the runtime-fallback path in discoverAudioVolumeAutomationFromTimeline at htmlCompiler.ts:2317 is arguably the more delicate of the two consumers (it's the one that would silently return null on getElementById("bed__hf2") if the bridge weren't installed). A one-liner audio test would pin the audio side of the invariant. Not blocking — the audio path shares its resolver code with video, and manual repro covers it.
• discoverAudioVolumeAutomationFromTimeline has no test that exercises the __hfMediaEl branch. Neither of the two new window.__hfMediaEl?.(id) ?? … sites (htmlCompiler.ts:2226, :2317) has a unit test that passes in a render id and verifies resolution — the existing tests at htmlCompiler.test.ts:2058+ and :2263+ predate this PR and use raw ids. Coverage-in-practice comes from discoverMediaFromBrowser (:107–117 in this PR) plus the compileForRender integration test (:2825), which is enough given the resolver is a single-line document.querySelector([${attr}="${id}"]). But a targeted volume-automation test with a colliding empty-src <audio> would close the loop.
• <img> fully-src-less is not covered. <img id="x"> (no src attribute at all) still skips stamping per MEDIA_SELECTOR = "video, audio, img[src]". The docstring justifies this as avoiding decorative-image stamping, which is reasonable, but if any author does document.getElementById("x").src = url on an <img> with no static src, the same #3512-shaped collision would resurface. Out of scope for this PR — worth a follow-up ticket if the surface exists.

Question

• Depth of nesting. The tests exercise depth-2 nesting (root → two scene hosts). Is there any known composition shape at ≥3 depth where the assignMediaRenderIds first-pass taken set would produce a suffix collision I'm missing? I don't see one — the loop is order-invariant on the existing-first pass and uniqueRenderId at mediaRenderIds.ts:83–88 handles arbitrary depth — but calling it out in case there's a producer path where the compiled document is a concatenation of independently-stamped sub-documents rather than one querySelectorAll sweep.

What I didn't verify

• Manual reproduction against ArcadeHQ/hyperframes-repros@patch/runtime-src-duplicate-ids. Val ran it (test-plan checkbox); I trusted the frame-level assertion in the issue and the CI regression shards passing.
• The bus/audio-group path is untouched here — I confirmed AUDIO_GROUP_SELECTOR still requires [id] (unchanged from before), and buses aren't the failure surface of #3512. If runtime-src is ever a thing on buses, that's a separate module.

Ready to ship from where I sit. Straightforward P0 close, tests in the right places, no observable seams.

— Review by Rames D Jusso

Approving. Read the four files plus the consumers at 874b323e; Rames D Jusso's review is the substantive first pass and I agree with it, so this is additive rather than a restatement.

The axis I checked independently: membership, not preference.

That review verifies every downstream consumer prefers the render id over the author id. The behavior change here is different in kind — dropping hasPlayableSource means elements that previously carried no attribute at all now carry one, and assignMediaRenderIds even mints a positional hf-media-${taken.size} for an element with no author id (mediaRenderIds.ts:114-118). So a decorative source-less <video> is newly stamped where before it was invisible. The question that raises is whether anything enumerates by the attribute and would now pick up phantom media.

Traced all four consumers the diff touches or implicates:

• discoverMediaFromBrowser enumerates video[data-start], audio[data-start], img[data-var-src] (htmlCompiler.ts:2140-2147) — gated on the timing compiler's attribute, not on data-hf-render-id.
• discoverVideoVisibilityFromTimeline enumerates video[data-hf-auto-start] (:2398) — same shape.
• discoverAudioVolumeAutomationFromTimeline is driven by a caller-supplied ids array, so it has no enumeration of its own.
• renderMediaCollector.ts:96 does enumerate [MEDIA_RENDER_ID_ATTR], but collectHostWindows builds a keyed lookup map. Extra keys are dead entries, never an output list.

So the widened stamp cannot introduce phantom clips on any path, and the hf-media-N fallback is unreachable in effect for the same reason — an element the timing compiler never processed has neither data-start nor data-hf-auto-start. Membership is safe.

Bridge chain — confirmed independently at source, since the two new window.__hfMediaEl?.(id) sites fail soft and a missing bridge would degrade silently (render id clip__hf2 → getElementById miss → clip dropped from volume automation, quieter than the bug being fixed): probeStage.ts:309 and :418 → createCaptureSession (frameCapture.ts:1145) → constructCaptureSessionWithRollback:1246 → constructCaptureSession:1286 → installMediaRenderIdBridge:1337, unconditional and before navigation. Same conclusion, reached separately.

PR-body claims spot-checked against the diff: "author ids are still not rewritten" holds — the loop only ever calls setAttribute(MEDIA_RENDER_ID_ATTR, …). "__hfMediaEl already installed on the probe page" holds per the chain above.

On CI, one correction to the record. Both relays here say "all green," and that is true — but only of each check's latest attempt. There are 97 runs across 50 names at this head, and earlier attempts of Test and regression are failure with most others cancelled. A naive per-name query returns those stale attempts. The rollup is SUCCESS and all eight ruleset-required checks (Semantic PR title, Test: runtime contract, Typecheck, Build, regression, Test, Render on windows-latest, Tests on windows-latest) are green on latest. Flagging because I mis-read it that way myself first — the re-runs are real and the summary is only true with "latest attempt" attached.

Nits in the prior review (audio-analogue test, untested __hfMediaEl branch, id-less <img>) are fair and non-blocking; I'd add the audio collision test as the one with real value, since the audio resolver is the fail-soft path.

— Rames